-
Notifications
You must be signed in to change notification settings - Fork 27.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Keep some buffered pages, that won't be disposed. Fix #1939 #2592
Keep some buffered pages, that won't be disposed. Fix #1939 #2592
Conversation
I really like the concept behind this PR. But I suggest to change it like this. Check this: https://github.com/zeit/next.js/blob/v3-beta/server/on-demand-entry-handler.js#L20 It keeps tracks of pages we've accessed. Currently it only keep a single entry. Initialize that with the Then do a include check here: https://github.com/zeit/next.js/blob/v3-beta/server/on-demand-entry-handler.js#L237 |
I managed to get a working version of it, but I have a problem with the tests! The way the |
b321f05
to
a041b70
Compare
a041b70
to
4d96366
Compare
@arunoda I went around the issues I had with the tests, and manage to make it work following your suggestion (or at least, I believed it's closer from what you envisioned). |
Hey @arunoda, is this feature still something you're interested in? Would you like me to change anything? I know that with the 3.0 release, you had a pretty busy time :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm going to merge this in 👌
* Revert "Keep some buffered pages, that won't be disposed. Fix #1939 (#2592)" This reverts commit 3643612. * Revert "Add beta installation instruction" This reverts commit 418cc21. * Revert "4.0.0-beta.1" This reverts commit c2d98e2. * Revert "Treat navigation to empty hash as hash navigate (#2971)" This reverts commit c6bd6ef. * Revert "React 16 (fiber) (#2996)" This reverts commit d19cc97.
Thanks a lot :) |
It's released under |
Sweet, any notes or additions to the readme on how to change pagesBufferLength from the default? |
Implementation of @arunoda's suggestion of letting the user decide how many page should be kept in memory before disposing them.
@sedubois made his point for not over disposing built pages, so I won't repeat him, but I think he has a point:
The implementation is pretty naive,
pagesBuffer
works as a queue (first in, first out) of max length two (or whatever the user set), we keep in the buffer the last requested pages. If you'd like me to encapsulate the pagesBuffer logic in, do not hesitate, I didn't want to add any non needed abstraction :)